fix(docs): Skip Fern bash-script tests on Windows#2017
Conversation
📝 WalkthroughWalkthroughThe PR adds Windows-specific test skipping to the docs link checker test suite. The ChangesConditional test skip on Windows
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes 🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Greptile SummaryThis PR adds
|
| Filename | Overview |
|---|---|
| tests/test_docs_links.py | Adds @pytest.mark.skipif(platform.system() == 'Windows', ...) to all five bash-backed tests; import platform and import pytest added. The guard works but shutil.which('bash') is None would be more portable. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["pytest collect test_docs_links.py"] --> B{"platform.system() == 'Windows'?"}
B -- Yes --> C["SKIP test\n(all 5 test functions)"]
B -- No --> D["run_link_check()\ncalls bash check-docs-links.sh"]
D --> E["assert returncode / output"]
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
tests/test_docs_links.py:44-45
Using `platform.system() == "Windows"` will skip tests even when `bash` is genuinely available on Windows (e.g. Git Bash, WSL), and conversely will not skip on non-Windows systems where `bash` is absent. Since `shutil` is already imported, checking `shutil.which("bash") is None` is a more portable condition and avoids a spurious skip on Windows environments that do have bash. The same replacement applies to all five decorators, and `import platform` can be removed once this is changed everywhere.
```suggestion
@pytest.mark.skipif(shutil.which("bash") is None, reason="bash not found in PATH")
def test_reports_broken_local_markdown_links_with_source_line_numbers(
```
Reviews (1): Last reviewed commit: "Skip fern docs tests using bash scripts ..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
Manually triggered a "Full Tests" run on this branch here. This confirms the Window Full Tests are clean after this merge. |
tgasser-nv
changed the title
2 participants
Description
The Full Tests which check both Windows and Linux only run nightly. The unrelated PR #2016 had failing Windows tests on this run. The errors are all in Fern and doc-related changes, introduced in #2015 . This PR wasn't blocked because the Full-Tests weren't run prior to merge.
The 5 test failures all stem from the same issue:
test_reports_broken_local_markdown_links_with_source_line_numbers
test_ignores_links_inside_inline_code_and_html_comments
test_resolves_guardrails_fern_routes
test_rejects_mdx_suffixes_for_links_that_resolve_as_fern_routes
test_fails_loudly_on_malformed_html_comments
All are calling bash to run check-docs-links.sh, which fails because WSL is not available on the Windows runner.
This PR is a temporary fix by removing these tests when running on Windows. However, our users with a Windows laptop would reasonably expect everything to work the same as on MacOS and Linux and this needs to be fixed for them.
Related Issue(s)
#2015 : The PR introducing the regression
#2017 : This PR temporarily disabling
tests/test_docs_links.pyunder Windows .Checklist
Summary by CodeRabbit